[codex] validate required Codex agent descriptions#175
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces validation for the required description frontmatter field in the codex-agents extension, ensuring it exits with a non-zero status and a clear error message when missing. It also updates the documentation and adds Go tests to verify this behavior. The review feedback suggests improving the validation check by trimming whitespace to prevent whitespace-only descriptions from bypassing the check.
| if (!frontmatter.description) { | ||
| throw new Error( | ||
| "codex-agents: missing required frontmatter 'description' (Codex custom agents require description)" | ||
| ); | ||
| } |
There was a problem hiding this comment.
If the description frontmatter field contains only whitespace (e.g., description: " "), the current check !frontmatter.description will evaluate to false and bypass the validation. This would result in generating a TOML file with an empty/whitespace description, which Codex cannot load. Trimming the value before checking ensures that whitespace-only descriptions are also rejected.
| if (!frontmatter.description) { | |
| throw new Error( | |
| "codex-agents: missing required frontmatter 'description' (Codex custom agents require description)" | |
| ); | |
| } | |
| if (!frontmatter.description || !frontmatter.description.trim()) { | |
| throw new Error( | |
| "codex-agents: missing required frontmatter 'description' (Codex custom agents require description)" | |
| ); | |
| } |
There was a problem hiding this comment.
Thanks, good catch. Updated the check to reject whitespace-only descriptions and added a test case for it.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds validation in the bundled codex-agents extension to require a description frontmatter field, since Codex custom agents cannot load without it. Errors thrown from mapFields are now caught and surfaced via stderr with a non-zero exit code.
Changes:
- Validate presence of non-empty
descriptionfrontmatter inconvert.jsand throw a descriptive error. - Add error handling to
md-toml.jsconvert()so thrown errors produce a clean stderr message and exit code 1. - Add Go tests exercising the bundled JavaScript extension via
node, and update documentation.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| extensions/codex-agents/convert.js | Adds required-field validation for description. |
| extensions/codex-agents/md-toml.js | Catches promise rejections, writes message to stderr, exits 1. |
| internal/sync/extension_test.go | Adds tests for successful conversion and missing/whitespace-only description. |
| extensions/README.md | Documents the pattern of validating required fields in convert.js. |
| website/docs/reference/commands/extras.md | Notes that the reference transform errors when description is missing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@PeterTianbuhan Thanks! Merged! |
Type
proposals/only — see CONTRIBUTING.md)Linked Issue
Related to #170
Summary
Codex TOML agents require a
descriptionfield. The bundledcodex-agentstransform previously skipped empty frontmatter values, so a Markdown agent withoutdescriptioncould sync successfully while producing TOML that Codex cannot load.This change makes that failure explicit:
extensions/codex-agentsnow rejects inputs withoutdescriptionfrontmatter, exits non-zero with a clear message, and documents the requirement in both the extension README and extras docs.Checklist
make check) — for code changesValidation
node -c extensions/codex-agents/convert.jsnode -c extensions/codex-agents/md-toml.jsname,description,model, anddeveloper_instructionswhendescriptionis present.missing required frontmatter 'description'whendescriptionis absent.I could not run
make checklocally because this Codex environment does not havego/gofmtinstalled.